Skip to content

Refactor WriteBackendMethods a bit#152801

Open
bjorn3 wants to merge 5 commits intorust-lang:mainfrom
bjorn3:lto_refactors14
Open

Refactor WriteBackendMethods a bit#152801
bjorn3 wants to merge 5 commits intorust-lang:mainfrom
bjorn3:lto_refactors14

Conversation

@bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Feb 18, 2026

No description provided.

@bjorn3 bjorn3 added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Feb 18, 2026
@rustbot
Copy link
Collaborator

rustbot commented Feb 18, 2026

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

@rustbot rustbot added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 18, 2026
@rustbot
Copy link
Collaborator

rustbot commented Feb 18, 2026

r? @jackh726

rustbot has assigned @jackh726.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 68 candidates
  • Random selection from 15 candidates

@rustbot

This comment has been minimized.

Sign in to view
@rust-bors

This comment has been minimized.

Sign in to view
@rustbot

This comment has been minimized.

Sign in to view
@bjorn3 bjorn3 mentioned this pull request Feb 24, 2026
Closed
13 tasks
@bjorn3
Copy link
Member Author

bjorn3 commented Feb 25, 2026

@rustbot ready

@bjorn3 bjorn3 removed the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Feb 25, 2026
@bjorn3 bjorn3 force-pushed the lto_refactors14 branch from 2bfa330 to 6ea5244 Compare March 3, 2026 15:26
@rustbot
Copy link
Collaborator

rustbot commented Mar 3, 2026

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

jackh726
jackh726 approved these changes Mar 3, 2026
View reviewed changes
Copy link
Member

@jackh726 jackh726 left a comment •
edited by rustbot
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small comment.

In general, as I've said previously, it would be helpful to have some context on why these changes are being made - particularly because this is not an area I'm really familiar with.

View changes since this review

compiler/rustc_codegen_llvm/src/lib.rs Outdated
let _profiler = TimeTraceProfiler::new(time_trace);
f()
})
fn thread_profiler() -> Box<dyn Any> {
Copy link
Member

@jackh726 jackh726 Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use Box<dyn Any>? I feel like it makes more sense to return TimeTraceProfiler, and then using Option<TimeTraceProfiler> at the call sites? Is there a reason that this needs to be on the heap?

Copy link
Member Author

@bjorn3 bjorn3 Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TimeTraceProfiler is defined in cg_llvm, while thread_profiler is part of a trait defined in cg_ssa.

Copy link
Member

@jackh726 jackh726 Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kk then it's fine as-is

@jackh726 jackh726 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 3, 2026
@bjorn3
Copy link
Member Author

bjorn3 commented Mar 3, 2026

In general, as I've said previously, it would be helpful to have some context on why these changes are being made - particularly because this is not an area I'm really familiar with.

As part of implementing rust-lang/compiler-team#908 I will have to move the LTO invocation from OngoingCodegen::join to the linker code (or perhaps slightly before it). To be able to do that, I think I may have to merge WriteBackendMethods into CodegenBackend or something like that (or maybe there could be a perform_lto method after join_codegen on CodegenBackend accepting a MaybeLtoModules?). To make this easier to do without issues for cg_clif (which doesn't use cg_ssa::write currently), I figured I would clean up WriteBackendMethods and move some things that can be moved out of OngoingCodegen::join already out of this method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jackh726 jackh726 jackh726 approved these changes

Assignees

@jackh726 jackh726

Labels

A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bjorn3 @rustbot @jackh726